Skip to content

Conversation

@guhetier
Copy link
Collaborator

@guhetier guhetier commented Dec 19, 2025

Description

When an endpoint receives a new stream indication, it can provide some receive buffers inline to convert the stream to app-owned buffer mode.
When doing so, the receive buffer struct of the stream is reinitialized, and the virtual buffer length (which correspond to the stream receive window) was set to zero.

This is incorrect since we introduced the "more buffer needed" notification and the receive window is no longer tied to the amount of buffer provided.

This would result in a receive failure with data received outside of the virtual size.

When converting the receive buffer to app-buffer mode, the virtual size is now preserved.

Fixes #5672.

Testing

Test didn't catch this issue because the amount of buffer provided were too big and a single datagram would not allow to go over the virtual size.

Adding more test case:

  • test all sender / receiver configurations
  • test with various buffers numbers and sizes

While this is a large churn of test code to implement the various scenarios, the tested logic is still the same:

  • provide some amount of receive buffer
  • send data
  • provide more receive buffers after some amount of data is received, or upon a buffer needed notification
  • validate all the data is received correctly

Documentation

Clarify documentation about the app provided receive buffer ownership.

@codecov
Copy link

codecov bot commented Dec 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.80%. Comparing base (7d4f45b) to head (6bcb9f9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5690      +/-   ##
==========================================
- Coverage   86.11%   84.80%   -1.31%     
==========================================
  Files          60       60              
  Lines       18710    18712       +2     
==========================================
- Hits        16112    15869     -243     
- Misses       2598     2843     +245     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@guhetier guhetier force-pushed the guhetier/fix_app_buffer branch from 47a91d3 to 3c44c74 Compare January 15, 2026 01:25
@guhetier guhetier force-pushed the guhetier/fix_app_buffer branch from 3c44c74 to 3405d87 Compare January 15, 2026 01:28
@guhetier guhetier marked this pull request as ready for review January 15, 2026 01:33
@guhetier guhetier requested a review from a team as a code owner January 15, 2026 01:33
@guhetier guhetier enabled auto-merge (squash) January 15, 2026 23:37
@guhetier guhetier merged commit 43959b6 into main Jan 16, 2026
580 of 585 checks passed
@guhetier guhetier deleted the guhetier/fix_app_buffer branch January 16, 2026 06:02
guhetier added a commit that referenced this pull request Jan 16, 2026
…fers #5690 (#5719)

## Description

When an endpoint receives a new stream indication, it can provide some
receive buffers inline to convert the stream to app-owned buffer mode.
When doing so, the receive buffer struct of the stream is reinitialized,
and the virtual buffer length (which correspond to the stream receive
window) was set to zero.

This is incorrect since we introduced the "more buffer needed"
notification and the receive window is no longer tied to the amount of
buffer provided.

This would result in a receive failure with data received outside of the
virtual size.

When converting the receive buffer to app-buffer mode, the virtual size
is now preserved.

Fixes #5672.

## Testing

Test not backported as they rely on changes present only in main

## Documentation

Clarify documentation about the app provided receive buffer ownership.

---------

Co-authored-by: Michael Friesen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StreamProvideReceiveBuffers doesn't invoke QUIC_STREAM_EVENT_RECEIVE_BUFFER_NEEDED on server end for a request

4 participants